Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Bump react-native version to fix build failures #1040

Merged
merged 17 commits into from
Nov 10, 2022

Conversation

gmaclennan
Copy link
Member

@gmaclennan gmaclennan commented Nov 7, 2022

DO NOT MERGE

On Nov 4th 2022 Facebook pushed a new version of react-native that broke react-native builds for everyone else. The suggested fix is to bump react-native from v0.66.4 to v0.66.5, which is what this PR does.

Fixes: #1039

However, this patch also seems to break react-native-v8, since running the app with this PR results in the following crash:

11-07 13:14:19.023 17803 17862 F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x109 in tid 17862 (mqt_js), pid 17803 (com.mapeo.debug)
11-07 13:14:19.063 17872 17872 E crash_dump32: failed to interrupt 17873 to detach: No such process
11-07 13:14:19.080 17872 17872 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
11-07 13:14:19.080 17872 17872 F DEBUG   : Build fingerprint: 'Android/sdk_phone_x86/generic_x86:9/PSR1.180720.012/4923214:userdebug/test-keys'
11-07 13:14:19.080 17872 17872 F DEBUG   : Revision: '0'
11-07 13:14:19.080 17872 17872 F DEBUG   : ABI: 'x86'
11-07 13:14:19.080 17872 17872 F DEBUG   : pid: 17803, tid: 17862, name: mqt_js  >>> com.mapeo.debug <<<
11-07 13:14:19.080 17872 17872 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x109
11-07 13:14:19.080 17872 17872 F DEBUG   : Cause: null pointer dereference
11-07 13:14:19.080 17872 17872 F DEBUG   :     eax 00000104  ebx c787c788  ecx 0000003e  edx 47fc5481
11-07 13:14:19.080 17872 17872 F DEBUG   :     edi c787c838  esi c22d0ea0
11-07 13:14:19.080 17872 17872 F DEBUG   :     ebp c787c778  esp c787c730  eip c880101e
11-07 13:14:19.084 17872 17872 F DEBUG   :
11-07 13:14:19.084 17872 17872 F DEBUG   : backtrace:
11-07 13:14:19.084 17872 17872 F DEBUG   :     #00 pc 00ef501e  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so (v8::internal::LookupIterator::State v8::internal::LookupIterator::LookupInRegularHolder<false>(v8::internal::Map, v8::internal::JSReceiver)+446)
11-07 13:14:19.084 17872 17872 F DEBUG   :     #01 pc 00ef295f  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so (v8::internal::LookupIterator::State v8::internal::LookupIterator::LookupInSpecialHolder<false>(v8::internal::Map, v8::internal::JSReceiver)+351)
11-07 13:14:19.084 17872 17872 F DEBUG   :     #02 pc 00ef210e  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so (void v8::internal::LookupIterator::Start<false>()+142)
11-07 13:14:19.084 17872 17872 F DEBUG   :     #03 pc 01033d96  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so (v8::internal::Runtime::GetObjectProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, bool*)+470)
11-07 13:14:19.084 17872 17872 F DEBUG   :     #04 pc 0103a595  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so
11-07 13:14:19.084 17872 17872 F DEBUG   :     #05 pc 00acd8f6  /data/app/com.mapeo.debug-ZGCcIhvtNt9b-qjZhWkcog==/lib/x86/libv8android.so

@gmaclennan gmaclennan force-pushed the fix/react-native-patch branch from 652fc4c to a80b874 Compare November 7, 2022 14:11
@gmaclennan
Copy link
Member Author

Update: using [email protected] seems to fix this and the app runs, @achou could you also test, and we'll see if CI works ok.

@gmaclennan gmaclennan marked this pull request as ready for review November 7, 2022 14:12
@achou11
Copy link
Member

achou11 commented Nov 7, 2022

@gmaclennan just tried this on my device and things seemed to work as expected. noticed CI is still failing though (not sure if due to flakiness or something more legitimate)

@gmaclennan
Copy link
Member Author

Looks like the app is not running/opening on CI at all, so need to fix that before we merge this.

@achou11
Copy link
Member

achou11 commented Nov 9, 2022

Summary of changes I added:

  • update jest cli to match jest version. this was preventing me from running the e2e tests locally. generally, i think it's supposed to match the jest version being used
  • upgrade detox to most recently published version (19.13.0). the release notes for 19 claim that it's not a breaking change for basic users, which i think we fall into. happy to revert if we're worried about this and want to minimize the reach of this PR
  • replace v8 with jsc intl. we needed to upgrade v8 to account for the React Native upgrade, but doing so caused Detox tests to not run properly. After some internal discussion, decided that the headaches aren't worth it for now and we should just use JSC for the sake of reducing maintenance complexity

@achou11
Copy link
Member

achou11 commented Nov 9, 2022

New issue to arise: Intl support for relative time.

Example error:

[Error: [React Intl Error FORMAT_ERROR] Error formatting relative time. 
Locale: TypeError: undefined is not an object (evaluating 'RelativeTimeFormat.bind')
MessageID: undefined
Default Message: undefined
Description: undefined 

]

Caused by: https://github.com/digidem/mapeo-mobile/blob/edc7115573fc3b58d689ee15c932d285dbef1be6/src/frontend/sharedComponents/DateDistance.js

Affected components:

  • PeerList
  • FormattedData
    • ObservationListView, ObservationShare, ObservationView, QuestionLabel, ObservationEditView, ObservationListItem, GPSModal

Solution: Need to polyfill via https://formatjs.io/docs/polyfills/intl-relativetimeformat/

@achou11
Copy link
Member

achou11 commented Nov 9, 2022

for posterity, can't believe i didn't find this sooner but this is why detox wasn't working with the v8 version we tried to upgrade to:

Kudo/react-native-v8#141

@achou11
Copy link
Member

achou11 commented Nov 9, 2022

Created a script to create the polyfill file for @formatjs/intl-relativetimeformat so i wouldn't have to write out the imports manually. not sure if it's entirely accurate so worth looking over

c4346c8

0270bd0

Startup time is notably slower, and not sure about bundle size increase, but let's evaluate and discuss as needed

scripts/build-polyfills.js Outdated Show resolved Hide resolved
src/frontend/polyfills/intl-relativetimeformat.ts Outdated Show resolved Hide resolved
const languages = require("../src/frontend/languages.json");

// For @formatjs/intl-relativetimeformat and @formatjs/intl-locale, which are needed for jsc-intl runtime
buildIntlRelativeTimeFormat();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the time of writing this, the output file looks like this:

// This file is automatically generated through scripts/build-intl-polyfills.js. Do not edit this directly!
// If you need to rebuild it, you can run `npm run build:intl-polyfills`
import "@formatjs/intl-locale/polyfill";
import "@formatjs/intl-relativetimeformat/polyfill";
import "@formatjs/intl-relativetimeformat/locale-data/de";
import "@formatjs/intl-relativetimeformat/locale-data/es";
import "@formatjs/intl-relativetimeformat/locale-data/fr";
import "@formatjs/intl-relativetimeformat/locale-data/id";
import "@formatjs/intl-relativetimeformat/locale-data/ja";
import "@formatjs/intl-relativetimeformat/locale-data/km";
import "@formatjs/intl-relativetimeformat/locale-data/my";
import "@formatjs/intl-relativetimeformat/locale-data/ne";
import "@formatjs/intl-relativetimeformat/locale-data/nl";
import "@formatjs/intl-relativetimeformat/locale-data/en";
import "@formatjs/intl-relativetimeformat/locale-data/pt";
import "@formatjs/intl-relativetimeformat/locale-data/si";
import "@formatjs/intl-relativetimeformat/locale-data/sw";
import "@formatjs/intl-relativetimeformat/locale-data/ta";
import "@formatjs/intl-relativetimeformat/locale-data/th";
import "@formatjs/intl-relativetimeformat/locale-data/vi";

package.json Outdated Show resolved Hide resolved
@achou11
Copy link
Member

achou11 commented Nov 10, 2022

Not sure how accurate this is, but index.android.bundle size from last successful CI run here is 1.46MB while v5.4.7 is 1.4MB (diff of 56.83 KB)

@achou11 achou11 merged commit 1c7ff1e into develop Nov 10, 2022
@achou11 achou11 deleted the fix/react-native-patch branch November 10, 2022 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failure to build due to Facebook/Meta issue
2 participants